Skip to content

Conversation

@tommy-gilligan
Copy link

To silence the deprecation warning mentioned at #346

So that it is backwards compatible with Rails versions < 6:

  • call module_parent if ActiveSupport version >= 6.0.0.beta1
  • otherwise call parent

@joevandyk
Copy link
Contributor

anything preventing this from being merged/released?

Copy link
Contributor

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks nice.

not sure if there are any 6.0 gemfiles to test, I'll try and add that if my first attempt at travis is passing.


def hierarchy_class_for_model
hierarchy_class = model_class.parent.const_set(short_hierarchy_class_name, Class.new(ActiveRecord::Base))
parent_class = if ActiveSupport.version >= Gem::Version.new('6.0.0.beta1')
Copy link
Contributor

@kbrock kbrock Sep 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may want to to_s for the comparison - it simplifies things

Also, rails 6.0 has been released, so probably want to fix that too

Suggested change
parent_class = if ActiveSupport.version >= Gem::Version.new('6.0.0.beta1')
parent_class = if ActiveSupport.version.to_s >= "6.0.0"


def hierarchy_class_for_model
hierarchy_class = model_class.parent.const_set(short_hierarchy_class_name, Class.new(ActiveRecord::Base))
parent_class = if ActiveSupport.version >= Gem::Version.new('6.0.0.beta1')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also be

Suggested change
parent_class = if ActiveSupport.version >= Gem::Version.new('6.0.0.beta1')
parent_class = if ActiveSupport::VERSION::MAJOR >= 6

Copy link
Contributor

@kbrock kbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this changed the gemfiles directly instead of changing the Appraisal file

gem "mysql2", "< 0.5"
gem "pg", "~> 0.21"
gem "sqlite3"
gem "sqlite3", "~> 1.3", "< 1.4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these gemfiles have been fixed at their source in appraisals.

Could you remove the changes to the gemfiles from this PR and ensure that the tests work for you?

gem "mysql2"
gem "pg"
gem "sqlite3"
gem "sqlite3", "~> 1.3", "< 1.4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, remove this one too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants